Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Basic implementation of Caching Middleware #39

Merged
merged 30 commits into from
Oct 20, 2023
Merged

Basic implementation of Caching Middleware #39

merged 30 commits into from
Oct 20, 2023

Conversation

evgeniy-scherbina
Copy link
Contributor

@evgeniy-scherbina evgeniy-scherbina commented Oct 9, 2023 •
edited
Loading

@evgeniy-scherbina evgeniy-scherbina changed the title Yevhenii/cache Basic implementation of Caching Middleware Oct 10, 2023
galxy25
galxy25 reviewed Oct 10, 2023
View reviewed changes
clients/cache/redis.go Outdated Show resolved Hide resolved
galxy25
galxy25 reviewed Oct 10, 2023
View reviewed changes
decode/evm_rpc.go Outdated Show resolved Hide resolved
galxy25
galxy25 reviewed Oct 10, 2023
View reviewed changes
service/cachemdw/cache.go Outdated Show resolved Hide resolved
galxy25
galxy25 suggested changes Oct 10, 2023
View reviewed changes
Copy link
Contributor

@galxy25 galxy25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in addition to the requested changes in the comments

  • Update developer docs with notes on what type of cache is used
  • Add new architecture docs for how the cache works, and how it modifies the proxy request flow, and how to debug it when something goes wrong
  • finish out todo’s
  • add docs on how to invalidate the cache (either all keys or for specific methods)
  • add e2e integration test in main_test.go that verifies when a cacheable request is sent to the proxy service it does get cached in the local docker redis

@galxy25
Copy link
Contributor

galxy25 commented Oct 12, 2023

@evgeniy-scherbina we'll want to be able to flip the cache on or off via an environment variable (e.g. some if block in those middleware that is checked first before running any of the caching specific code and hands off to the next handler if the cache is configured to be disabled) in cases where there is a bug that is safer / faster to bypass in production by disabling the cache, and to allow debugging to correctly triage if any observed issues with the proxy are related to the caching code paths

@evgeniy-scherbina evgeniy-scherbina force-pushed the yevhenii/cache branch 5 times, most recently from dfaec36 to d0868b8 Compare October 13, 2023 20:55
@evgeniy-scherbina evgeniy-scherbina force-pushed the yevhenii/cache branch 2 times, most recently from ed69b88 to 7066f92 Compare October 19, 2023 14:02
@evgeniy-scherbina evgeniy-scherbina force-pushed the yevhenii/cache branch 2 times, most recently from 5e6ad5e to 589b2e4 Compare October 19, 2023 15:52
galxy25
galxy25 reviewed Oct 19, 2023
View reviewed changes
config/config.go Outdated Show resolved Hide resolved
@evgeniy-scherbina
Copy link
Contributor Author

@evgeniy-scherbina any idea why the github actions e2e and unit tests aren't running for this PR?

there were conflicts

pirtleshell
pirtleshell approved these changes Oct 20, 2023
View reviewed changes
service/cachemdw/cache.go Show resolved Hide resolved
@evgeniy-scherbina evgeniy-scherbina merged commit 956b227 into main Oct 20, 2023
4 checks passed
@evgeniy-scherbina evgeniy-scherbina deleted the yevhenii/cache branch October 20, 2023 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pirtleshell pirtleshell pirtleshell approved these changes

@galxy25 galxy25 galxy25 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@evgeniy-scherbina @galxy25 @pirtleshell